fix(BACKEND-ATTN-REGISTRY): a platform reports an SM version or nothing, and Vulkan was refusing FLASH_ATTN too (#1823) - #1834
Merged
Conversation
…ng, and Vulkan was refusing FLASH_ATTN too (#1823) `FlashAttentionBackend::supports_compute_capability` is upstream's NVIDIA `capability >= (8, 0)` (flash_attn.py:200-202). `MetalPlatform::get_device_capability` answered with the MTLGPUFamilyApple generation and `VulkanPlatform` with the Vulkan API version, so an SM-version bar was compared against two numbers that are not SM versions. Apple family 9 on the M4 gate box cleared it by coincidence; a GitHub `macos-15` runner reported lower and `macos-metal-mlx` has been red since 369ea7f. The oracle is unambiguous and had already met this case. `Platform.get_device_capability` is documented as the "stateless version of torch.cuda.get_device_capability" (interface.py:420-431), `validate_configuration` calls the predicate with no None guard (backend.py:366-367) because its only callers are CudaPlatform (cuda.py:381,410, after `assert device_capability is not None` at :404) and RocmPlatform (rocm.py:531,558), and every other platform selects its backend without ever evaluating it (cpu.py:75-87). When upstream hit a platform whose capability format differs from CUDA's it returned None and said why: xpu.py:228-236, "capacity format differs from cuda's and will cause unexpected failure". Metal and Vulkan now answer the same way. The Apple family and the Vulkan API version stay on `vt::Backend`, which is the seam that owns device-unit questions, and the tests assert them there so this is not a number deleted to make a gate pass. Grounding the premise turned up a second live instance. Vulkan names FLASH_ATTN as the only entry in its priority list while reporting `major == 1`, which is true of every Vulkan device that will ever exist, so `SelectAttentionBackendName` threw on kVULKAN unconditionally. Nothing saw it because the lane's test asserted that FLASH_ATTN is NAMED in the priority list and never that the selector REACHES it — the same gap the Metal test had. That gap is what made the defect locally reproducible without a Mac, and the red-before below is a Vulkan run. Measured, this moves two device types rather than the four the issue estimated: kCPU and kTENSTORRENT already report an absent capability and do not move at all, and kCUDA/kROCM answer in the unit the predicate expects. No selection rework was needed. RED-BEFORE, lavapipe, base df1ee20, assertion added and no fix applied: `present=true major=1 minor=4`, then `CHECK( SelectAttentionBackendName(p) == "FLASH_ATTN" ) THREW "No valid attention backend for device type 3 from {FLASH_ATTN: [compute capability not supported]}"` — the same message test_metal_backend.cpp:170 produced for device type 2. GREEN-AFTER: `present=false major=-1 minor=-1; vt::Backend API version 1.4`, selector resolves. test_vulkan_backend 35/35, 2109 assertions, SUCCESS!, 0 skipped; test_backend_cross_device 25/25, 80140 assertions, SUCCESS!; test_platform on the Vulkan tier 15/15, 118 assertions, SUCCESS!. Restoring the defective Vulkan body reds both new gates and the tree restores byte-identical. `test_platform` gains the contract as a class gate rather than a list of platforms, so a platform added later is covered with no edit, and the Vulkan lane now runs it — on build-test-cpu only kCPU is registered, which cannot catch the defect the case exists for. Closes #1823 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…e, and two seam headers stop teaching the retired model (#1823) Repairs the fresh review of the previous commit, which passed the fix on correctness and failed it on the reach the new gate claimed. A device-less Vulkan run passed everything while measuring nothing. `test_vulkan_backend.cpp:442` opens with a silent `if (!VulkanPresent()) return;`, so with the loader pointed at a missing ICD `test_platform` reports 15/15 and `Status: SUCCESS!` having walked kCPU alone, and the platform case reports `test cases: 1 | 1 passed` with `assertions: 0 | 0 passed` — a skip wearing a pass, under a green job. A test cannot tell "no device on this runner" from "CPU tier", but the lane knows which one it is, so the positive control belongs in the lane: both `build-test-vulkan` steps now grep the case's own MESSAGE line, and against those device-less logs the greps return rc 1 while the executables return rc 0. The class gate also did not run on the Metal tier at all, and `tests/vllm/platforms/test_platform.cpp` said it did. `macos-metal-mlx` built `vllm test_metal_backend` and ran only the suite, so "a platform added later is covered with no edit here" was false for the one accelerator tier that is not Vulkan. The lane now builds and runs `test_platform` with the same positive control for `platform metal`, which makes the claim true rather than deleting it. `src/vt/vulkan/vulkan_context.h` and `src/vt/metal/metal_context.h` still told the reader that the API version and the Apple family are "mirrored onto the Platform seam", and that `has_device_capability(1, 1)` reads as "Vulkan >= 1.1". That is the retired model, in the two headers that own the numbers, cross- referencing the two files this row corrected — the same defect class the row exists to repair. Four stale `interface.py` anchors are corrected against the pin. The spec gains the oracle anchor the review found and the implementer missed: `mla/prefill/selector.py:97-102 get_mla_prefill_backend` is upstream's own PLATFORM-AGNOSTIC selector, and on an absent capability it skips every capability predicate and returns FLASH_ATTN. That is the closest structural mirror of `SelectAttentionBackendName` upstream has, and it settles the design independently of the caller-guarantee argument. Its `## Evidence` mutation figure is corrected from 21 insertions / 7 deletions to 4 / 1: the original was measured against `origin/main` before the fix was committed, so it carried the whole change and could not show that the mutation applied. Re-verified on lavapipe: compile rc 0, test_vulkan_backend 35/35 and 2109 assertions SUCCESS! with 0 skipped, test_backend_cross_device 25/25 and 80140 assertions SUCCESS!, test_platform 15/15 and 118 assertions SUCCESS!, and each new ci.yml grep run verbatim at rc 0 with the device present and rc 1 without it. `src/vllm/platforms/metal.cpp` and `tests/vt/test_metal_backend.cpp` both pass `g++ -fsyntax-only -std=c++20` at rc 0. `scripts/agent-preflight.sh --staged` rc 0; the one earlier failure was `test_cpu_x86_llamacpp_floor` at load average 17, which passes 10/10 re-run and is #618. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
The dispatched run on this branch reported `agent-record` red on `FileNotFoundError: [Errno 2] No such file or directory: 'hugo'` from `tests/scripts/test_check_site.py:130`. That is not this row's finding: it is #1754, repaired on `main` by `4f0d44ca2` (#1830), which this branch predated by two commits. Merging rather than rebasing because rebasing this branch would need a force-push, and nothing here may be force-pushed. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…im findings (#1823) The re-review of the repair commit returned PASS with four LOW findings. None weakened a gate; all four were statements that do not hold. `src/vt/vulkan/vulkan_context.h:302` had replaced one false claim with a narrower one. "Exposed on vt::Backend and NOWHERE ELSE" is not true of Vulkan: `api_major()/api_minor()` are public on `VulkanContext`, the suite reads them directly, and `src/vllm/platforms/vulkan.cpp:48` says so in the same commit range. It now names both readers. The Metal twin at `metal_context.h:85` is left as written, because `gpu_family_apple()` genuinely has one consumer. The anchor sweep had stopped short. `src/vllm/platforms/platform.cpp:13,24` carried `interface.py:417-439` and `441-476` for the two functions whose DECLARATIONS were corrected in `interface.h`, and a third copy sat in a `TEST_CASE` name in a file this change already edits. The corrected `is_device_capability_family` anchor also overshot: the function is `interface.py:481-493`, and `481-495` runs into the next `@classmethod`. Six comments still wrote `xpu.py:228-236` while the spec had been corrected to `228-234`. The spec claimed the first dispatched run "reports `cancelled` for every job". It cancelled 12; 4 finished `success` and 3 `skipped`. The substantive point is unchanged — `macos-metal-mlx` is among the cancelled, so that run decides nothing — but a figure that does not reproduce is a figure that should not be in an evidence section. The spec also gains the repair head's own gate, read at job and step level on run 32683588981 at b3cc130: `macos-metal-mlx` success with all ten steps success, the new step 10 printing `platform metal get_device_capability() present=false major=-1 minor=-1 sm_unit=false` beside `platform cpu`, 15 cases / 116 assertions / SUCCESS!. kMETAL registers only when a device is probed, so that line is the contract measured on Apple hardware rather than inferred from a Linux syntax check. Re-verified: compile rc 0, test_platform 15/15 and 118 assertions SUCCESS! with 0 skipped, test_vulkan_backend 35/35 and 2109 assertions SUCCESS! with 0 skipped, the renamed case reachable by `-tc` at 1/1, and both Metal files at `g++ -fsyntax-only` rc 0. `scripts/agent-preflight.sh --staged` rc 0. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
Routine integration before landing the capability-unit fix. No conflicts. FOLLOWING_AGENTS_PROTOCOL Issue: #1823 Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FlashAttentionBackend::supports_compute_capabilityis upstream's NVIDIAcapability >= (8, 0)(flash_attn.py:200-202).MetalPlatform::get_device_capabilityanswered with the MTLGPUFamilyApple generation and
VulkanPlatformwith theVulkan API version, so an SM-version bar was compared against two numbers that
are not SM versions. Apple family 9 on the M4 gate box cleared it by
coincidence; a GitHub
macos-15runner reported lower andmacos-metal-mlxhas been red since 369ea7f.
The oracle is unambiguous and had already met this case.
Platform.get_device_capabilityis documented as the "stateless version of torch.cuda.get_device_capability"
(interface.py:420-431),
validate_configurationcalls the predicate with noNone guard (backend.py:366-367) because its only callers are CudaPlatform
(cuda.py:381,410, after
assert device_capability is not Noneat :404) andRocmPlatform (rocm.py:531,558), and every other platform selects its backend
without ever evaluating it (cpu.py:75-87). When upstream hit a platform whose
capability format differs from CUDA's it returned None and said why:
xpu.py:228-236, "capacity format differs from cuda's and will cause unexpected
failure". Metal and Vulkan now answer the same way. The Apple family and the
Vulkan API version stay on
vt::Backend, which is the seam that ownsdevice-unit questions, and the tests assert them there so this is not a number
deleted to make a gate pass.
Grounding the premise turned up a second live instance. Vulkan names FLASH_ATTN
as the only entry in its priority list while reporting
major == 1, which istrue of every Vulkan device that will ever exist, so
SelectAttentionBackendNamethrew on kVULKAN unconditionally. Nothing saw it because the lane's test
asserted that FLASH_ATTN is NAMED in the priority list and never that the
selector REACHES it — the same gap the Metal test had. That gap is what made the
defect locally reproducible without a Mac, and the red-before below is a Vulkan
run.
Measured, this moves two device types rather than the four the issue estimated:
kCPU and kTENSTORRENT already report an absent capability and do not move at
all, and kCUDA/kROCM answer in the unit the predicate expects. No selection
rework was needed.
RED-BEFORE, lavapipe, base df1ee20, assertion added and no fix applied:
present=true major=1 minor=4, thenCHECK( SelectAttentionBackendName(p) == "FLASH_ATTN" ) THREW "No valid attention backend for device type 3 from {FLASH_ATTN: [compute capability not supported]}"— the same messagetest_metal_backend.cpp:170 produced for device type 2. GREEN-AFTER:
present=false major=-1 minor=-1; vt::Backend API version 1.4, selectorresolves. test_vulkan_backend 35/35, 2109 assertions, SUCCESS!, 0 skipped;
test_backend_cross_device 25/25, 80140 assertions, SUCCESS!; test_platform on
the Vulkan tier 15/15, 118 assertions, SUCCESS!. Restoring the defective Vulkan
body reds both new gates and the tree restores byte-identical.
test_platformgains the contract as a class gate rather than a list ofplatforms, so a platform added later is covered with no edit, and the Vulkan
lane now runs it — on build-test-cpu only kCPU is registered, which cannot catch
the defect the case exists for.
Closes #1823
What the fresh review sent back
Repairs the fresh review of the previous commit, which passed the fix on
correctness and failed it on the reach the new gate claimed.
A device-less Vulkan run passed everything while measuring nothing.
test_vulkan_backend.cpp:442opens with a silentif (!VulkanPresent()) return;,so with the loader pointed at a missing ICD
test_platformreports 15/15 andStatus: SUCCESS!having walked kCPU alone, and the platform case reportstest cases: 1 | 1 passedwithassertions: 0 | 0 passed— a skip wearing apass, under a green job. A test cannot tell "no device on this runner" from "CPU
tier", but the lane knows which one it is, so the positive control belongs in the
lane: both
build-test-vulkansteps now grep the case's own MESSAGE line, andagainst those device-less logs the greps return rc 1 while the executables return
rc 0.
The class gate also did not run on the Metal tier at all, and
tests/vllm/platforms/test_platform.cppsaid it did.macos-metal-mlxbuiltvllm test_metal_backendand ran only the suite, so "a platform added later iscovered with no edit here" was false for the one accelerator tier that is not
Vulkan. The lane now builds and runs
test_platformwith the same positivecontrol for
platform metal, which makes the claim true rather than deleting it.src/vt/vulkan/vulkan_context.handsrc/vt/metal/metal_context.hstill toldthe reader that the API version and the Apple family are "mirrored onto the
Platform seam", and that
has_device_capability(1, 1)reads as "Vulkan >= 1.1".That is the retired model, in the two headers that own the numbers, cross-
referencing the two files this row corrected — the same defect class the row
exists to repair. Four stale
interface.pyanchors are corrected against the pin.The spec gains the oracle anchor the review found and the implementer missed:
mla/prefill/selector.py:97-102 get_mla_prefill_backendis upstream's ownPLATFORM-AGNOSTIC selector, and on an absent capability it skips every capability
predicate and returns FLASH_ATTN. That is the closest structural mirror of
SelectAttentionBackendNameupstream has, and it settles the designindependently of the caller-guarantee argument. Its
## Evidencemutation figureis corrected from 21 insertions / 7 deletions to 4 / 1: the original was measured
against
origin/mainbefore the fix was committed, so it carried the wholechange and could not show that the mutation applied.
Re-verified on lavapipe: compile rc 0, test_vulkan_backend 35/35 and 2109
assertions SUCCESS! with 0 skipped, test_backend_cross_device 25/25 and 80140
assertions SUCCESS!, test_platform 15/15 and 118 assertions SUCCESS!, and each
new ci.yml grep run verbatim at rc 0 with the device present and rc 1 without it.
src/vllm/platforms/metal.cppandtests/vt/test_metal_backend.cppboth passg++ -fsyntax-only -std=c++20at rc 0.scripts/agent-preflight.sh --stagedrc 0; the one earlier failure was
test_cpu_x86_llamacpp_floorat load average17, which passes 10/10 re-run and is #618.
What the re-review sent back
The re-review of the repair commit returned PASS with four LOW findings. None
weakened a gate; all four were statements that do not hold.
src/vt/vulkan/vulkan_context.h:302had replaced one false claim with anarrower one. "Exposed on vt::Backend and NOWHERE ELSE" is not true of Vulkan:
api_major()/api_minor()are public onVulkanContext, the suite reads themdirectly, and
src/vllm/platforms/vulkan.cpp:48says so in the same commitrange. It now names both readers. The Metal twin at
metal_context.h:85is leftas written, because
gpu_family_apple()genuinely has one consumer.The anchor sweep had stopped short.
src/vllm/platforms/platform.cpp:13,24carried
interface.py:417-439and441-476for the two functions whoseDECLARATIONS were corrected in
interface.h, and a third copy sat in aTEST_CASEname in a file this change already edits. The correctedis_device_capability_familyanchor also overshot: the function isinterface.py:481-493, and481-495runs into the next@classmethod. Sixcomments still wrote
xpu.py:228-236while the spec had been corrected to228-234.The spec claimed the first dispatched run "reports
cancelledfor every job".It cancelled 12; 4 finished
successand 3skipped. The substantive point isunchanged —
macos-metal-mlxis among the cancelled, so that run decidesnothing — but a figure that does not reproduce is a figure that should not be in
an evidence section.
The spec also gains the repair head's own gate, read at job and step level on
run 32683588981 at b3cc130:
macos-metal-mlxsuccess with all ten stepssuccess, the new step 10 printing
platform metal get_device_capability() present=false major=-1 minor=-1 sm_unit=falsebesideplatform cpu, 15 cases /116 assertions / SUCCESS!. kMETAL registers only when a device is probed, so
that line is the contract measured on Apple hardware rather than inferred from a
Linux syntax check.
Re-verified: compile rc 0, test_platform 15/15 and 118 assertions SUCCESS! with
0 skipped, test_vulkan_backend 35/35 and 2109 assertions SUCCESS! with 0
skipped, the renamed case reachable by
-tcat 1/1, and both Metal files atg++ -fsyntax-onlyrc 0.scripts/agent-preflight.sh --stagedrc 0.FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]